@DanTup Thanks for the PR.

Could you please:

• rename your commit message to something that reads well as a one-liner in a change log, using the fix prefix. someting like:

fix: unhandled chokidar errors causes karma crash

• fix the styles (see travis build report)
• and, have you considered adding a unit tests for this? a tests that would fail when the line you've added is not there. it("should log chokidar errors")

LGTM (after processing @sylvain-hamel 's comments).

Thanks @DanTup

I'm struggling to figure out how to test this. I've looked in both file_list specs and watcher specs but I can't find any similar tests that are testing watcher's interaction with chokidar.

There are some tests in file_list that look like they're interacting with a mock filesystem (there's even a test it 'should handle fs.stat errors'), but this doesn't look like it's via chokidar.

Since my fix is in watcher, I presume my test belongs in the watcher specs, but none of the current specs interact with a filesystem or chokidar.

My best guess is that I need to add the mock fs to the watcher specs, and force it to error on stat (similar to the file_list specs), but I don't know if I can even make Chokidar use a mock fs. There's a mock chokidar in node_modules\mocks but it seems to have almost no implementation/hooks (and I presume it's from another project?); so I don't know where to begin trying to add this.

I'll update this PR with the other changes (style + commit message); but I'll need some guidance on getting a test in for this :(

(Also FYI; grunt lint fails on Windows with a bunch of "Invalid line break" messages I can't figure out. Your .gitattributes marks everything as text, which I believe should stop Git messing with newlines on Windows; yet I'm seeing it reported on tons of files I haven't even opened)

@DanTup ur specs shld best fit here

@deepak1556 That's where I started adding tests; but none of the existing ones seem to interact with any chokidar events; so I'm unsure how best to do it. I need to force choikdar to throw (eg. tell it a file has modified that doesn't exist, so it throws when trying to stat the file).

@DanTup regarding #993, after you change the settings on your pc, make sure you reinitialize your working folder for the new setting to apply.

Even following those instructions and re-cloning, I still get the same lint issue.

If I type git config core.autocrlf is doesn't output anything (it did before I unset it). I then deleted all files from the working folder, and did git reset --hard then npm install and grunt build then grunt lint. Same errors :(

It's only happening on some lines, which makes me wonder if the source files are bad, and those not seeing it are having them normalised on the way out?

(I can move this lint stuff to another Issue to keep this PR on-topic if you want?)

Yup a separate issue is better for lint. chokidar events needs to be supported on node-mocks, but i would like to hear @vojtajina approach on this.

Raised #1030 for lint issue; will await further guidance for this PR on how best to deal with testing chokidar interaction.

I'm ok with this without a unit test. The watch fn contains basically assembling all the pieces together and it's currently not unit tested...

You could extract the wiring of chokidar with FileList into a separate function and then unit test it. (you could test that 1/ the events are forwarded to FileList, 2/ that the FileList calls are with the right context, 3/ that Chokidar errors are ignored/logged, etc...)

It sounds like a bigger change than I think I could do well with my limited knowledge of all this; but I'm happy to come back and add a test if this "plumbing" for testing it is added later :)

This has been manually tested well; removing the handler causes karma to intermittently crash and vanish when saving and adding it in stops the crash (I confirmed the chokidar error was still occurring by logging when it raised errors, to ensure it wasn't just fluke).